Skip to content

Conversation

CraftSpider
Copy link
Contributor

@CraftSpider CraftSpider commented Nov 11, 2020

The simplest of bad rust-call definitions will no longer cause an ICE. There is a FIXME added for future work, as I wanted to get this easy fix in before trying to either add a hack or mess with the whole obligation system

fixes #22565

@rust-highfive
Copy link
Contributor

r? @oli-obk

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 11, 2020
@jyn514 jyn514 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 11, 2020
@CraftSpider
Copy link
Contributor Author

Note: overloaded-calls-nontuple is failing due to checking a case that is now being explicitly forbidden at the implementation location instead of later on. How should that be handled?

@oli-obk
Copy link
Contributor

oli-obk commented Nov 12, 2020

overloaded-calls-nontuple is failing due to checking a case that is now being explicitly forbidden at the implementation location instead of later on.

I'm not sure I understand, can you elaborate? I would have thought the failure is because you call let item = tcx.hir().expect_item(id); and methods are impl-items and not items.

@CraftSpider
Copy link
Contributor Author

Nevermind, you're right, I misread the error. I'll fix that, and have a go at moving it into check_fn.

@CraftSpider
Copy link
Contributor Author

Having fixed the ICE, now I'm getting the issue I mentioned, where the expected error is emitted, but so are 2 other errors saying A function with the "rust-call" ABI must take a single non-self argument that is a tuple. Is this valid to bless, as this is in fact an error as it stands? (Prior this commit, if you change the code to S::call_once(s, 3), you get the error this is meant to fix, so it is a valid emission of this new error)

@oli-obk
Copy link
Contributor

oli-obk commented Nov 17, 2020

Is this valid to bless, as this is in fact an error as it stands? (Prior this commit, if you change the code to S::call_once(s, 3), you get the error this is meant to fix, so it is a valid emission of this new error)

Yes, feel free to just bless all error changes. Especially after fixing an ICE this is very normal that more errors occur.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 18, 2020

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 18, 2020

📌 Commit e8426a6 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 18, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Nov 19, 2020
Make bad "rust-call" arguments no longer ICE

The simplest of bad rust-call definitions will no longer cause an ICE. There is a FIXME added for future work, as I wanted to get this easy fix in before trying to either add a hack or mess with the whole obligation system

fixes rust-lang#22565
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Nov 19, 2020
Make bad "rust-call" arguments no longer ICE

The simplest of bad rust-call definitions will no longer cause an ICE. There is a FIXME added for future work, as I wanted to get this easy fix in before trying to either add a hack or mess with the whole obligation system

fixes rust-lang#22565
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 19, 2020
Rollup of 14 pull requests

Successful merges:

 - rust-lang#78961 (Make bad "rust-call" arguments no longer ICE)
 - rust-lang#79082 (Improve the diagnostic for when an `fn` contains qualifiers inside an `extern` block.)
 - rust-lang#79090 (libary: Forward compiler-builtins "asm"  and "mangled-names" feature)
 - rust-lang#79094 (Add //ignore-macos to pretty-std-collections.rs)
 - rust-lang#79101 (Don't special case constant operands when lowering intrinsics)
 - rust-lang#79102 (Add two regression tests)
 - rust-lang#79110 (Remove redundant notes in E0275)
 - rust-lang#79116 (compiletest: Fix a warning in debuginfo tests on windows-gnu)
 - rust-lang#79117 (add optimization fuel checks to some mir passes)
 - rust-lang#79147 (Highlight MIR as Rust on GitHub)
 - rust-lang#79149 (Move capture lowering from THIR to MIR)
 - rust-lang#79155 (fix handling the default config for profiler and sanitizers)
 - rust-lang#79156 (Allow using `download-ci-llvm` from directories other than the root)
 - rust-lang#79164 (Permit standalone generic parameters as const generic arguments in macros)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5a58b50 into rust-lang:master Nov 19, 2020
@rustbot rustbot added this to the 1.50.0 milestone Nov 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ICE when simplest rust-call method is called
6 participants